-
Notifications
You must be signed in to change notification settings - Fork 196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
yamldot: Dotted-path YAML manipulation #4465
base: main
Are you sure you want to change the base?
Conversation
d08c151
to
c1f1db8
Compare
// Examples of dotted-path syntax: | ||
// - a.object.key | ||
// - b.item_list[1] | ||
package yamlnode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just FYI, I wrote this after not finding any suitable libraries out there that we could benefit from directly:
- There is an existing go-yaml library that supports more advanced notation, but this is a rewrite of YAML library. Don't feel strongly to push it through at this stage.
- There is yaml-jsonpath from VMWare. It supports more advanced notation for filtering. I'm not fully confident here with integrating this at this stage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm asuming the expectation from the consuption side is to call Encode( someYaml )
to get the yaml.node abstracion and then work directly with that object to set, find, append, etc. (using dotted-path syntax) and then write the ouput yaml at any moment.
Can we just use the Config
interface we have already, which works with map[string]any
?
The expectation would be to parse a YAML file into a Config
object. The config object already supports dottet-notation. Then, when you want a YAML back, you would call Raw() and use the YAML library to produce the yaml out of it.
I think that's what we are doing for JSON (for azd config) and should be the same for YAML
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would lose important AST metadata (comments, whitespacing) that we'd want to preserve while unmarshaling a YAML structure into any different type, including Config
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use case here is to programmatically manipulate an existing YAML file. While doing this, we want to preserve as much of the file content as possible. Perhaps the only acceptable thing for us to do is to format the file in a consistent way, like gofmt
. Otherwise, we should preserve as much of the existing file as possible.
For these reasons, we want to interact with YAML at the AST layer (syntax) and not at the semantic layer.
The other thing I'd mention is that in our defacto library for yaml
, Unmarshal
will not preserve items like binary data, and multiline blocks. These items of yaml
are also captured here as part of the yamlfmt
tool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this is enough reason to move the resources definition out of Azure.yaml
I don't think it is a good idea, neither a good design/practice, to have a file that is both auto-generated
and manually-human-created
. Usually, whatever that is system-autogenerated has a title about it and it's not expected to be manually manipulated.
It looks like it might be appreciated by customers, but I think it is a ticket for easy trouble and bugs.
For example, after customers run azd init
to generate azure.yaml, it is not expected that if you run azd init
again, azd would save any changes you previously added to azure.yaml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not expected that if you run azd init again, azd would save any changes you previously added to azure.yaml.
Good point, but that isn't the design being proposed here, we won't be regenerating files from scratch. We are modifying an existing file with an azd add
gesture.
to have a file that is both auto-generated and manually-human-created. Usually, whatever that is system-autogenerated has a title about it and it's not expected to be manually manipulated.
Do you feel that any CLI configuration file that can be manually edited inside the file, as well as via it's CLI config
command to be an exception to this rule?
Another popular example here that comes to mind is VSCode's UI-based editor that builds on top of an existing settings.json
file. A programmatic layer built on top of a file layer.
Also, consider go.mod
-- it's a configuration file that is auto generated, can be manually edited, and programmatically updated as well.
I wonder if this is enough reason to move the resources definition out of Azure.yaml
For various reasons, I wouldn't recommend this. In my mind, our north star goal is to create a complete "app model" in azure.yaml. Splitting the file wouldn't necessarily move us in that direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving a potential suggestion here: if we're unhappy about having these types of lower-level libraries in azd, I'm also more than happy to tender this as a personal library so that we shift the maintenance burden elsewhere.
The thing I'd point out is that the deeper we can integrate with YAML as a language (which we will have to do by simply supporting AKS), the more feature-heavy rich experiences we can offer. For that reason, I would recommend us developing technologies in this area.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My push is not about the new library, but the expected experience.
In the mentioned examples, since it is JSON, comments are not allowed. And if you manually add extra spaces to it, it is re-formated as soon you use the VSCode settings UI. I think it can even change the ordering of the config.
In that model, the user needs to adjust to how the system expect those files, never the other way around.
I'm OK with expecting folks to manually update azure.yaml, but I am not happy creating the expectation for Folks that the system will also update the file and keep all the details, like ordering, style, spaces, comments, etc. etc.
As a user, I know that I can manually update system/app files, but I need to make sure that I don't break anything and the only thing that I expect is that it makes the system/app to work the way I want. I don't care about maintaining the config file with my own style
Add functionality for YAML node manipulation using dotted-path syntax. This will be used in upcoming work that is aimed to manipulate YAML nodes.
c1f1db8
to
2408013
Compare
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you provide a use case that this solves vs the behavior that exists in our current usage of yaml?
@wbreza Responded to @vhvb1989 question similarly here. The use case to programmatically manipulate a YAML file while preserving the majority of the file content.
|
Add functionality for YAML node manipulation using dotted-path syntax.
This will be used in upcoming work that is aimed to manipulate YAML nodes.